Refactor tests to use explicit fixtures #44
Conversation
While this is just a quick draft, these tests use rustfix as a library and showcase how it might be used in clippy's test suite to check that suggestions can be applied. (It could e.g. work as an extension to compiletest.)
Just run it with `env RUSTFIX_TEST_RECORD_JSON=true cargo test`
One of my favorites! Isn't it great that clippy use get rind of one `&` per clippy run?
There are tests with multiple JSON documents (one per line)
Apparently, it's hard to figure out if files were added/deleted to a directory in a build script. Oh well.
| pub mod diagnostics; | ||
| use diagnostics::{Diagnostic, DiagnosticSpan}; | ||
|
|
||
| pub fn get_suggestions_from_json(input: &str, only: &HashSet<String>) -> Vec<Suggestion> { |
There was a problem hiding this comment.
Should note that this expects rustc's JSON output. Cargo puts each document in a {"message": rustc_doc} wrapper
| trace!("{:?}", sol); | ||
| for r in sol.replacements { | ||
| info!("replaced."); | ||
| trace!("{:?}", r); |
There was a problem hiding this comment.
Useful for debugging: env RUST_LOG=everything=trace
|
r? @oli-obk I hope I have some time over the next weekends to continue to hack on this and maybe even adopt it as an addition to compiletest if that's something you'd like to see. As rustfix depends on serde, and serde uses proc-macros, it sadly won't be (My not-so-secret goal is to show the vscode-rust plugin fixing clippy lints at FOSDEM. For that, I primarily need a list of lints whose suggestions work as autofixes and some time to hack on RLS to load the clippy plugin.) cc @Manishearth re: usage in clippy |
|
This is pretty neat! Working this into compiletest would be great for clippy. I don't have an explicit list of lints for you, but it would be nice to get rust-lang/rust#39254 implemented (it's pretty straightforward given that nothing is stabilized) so that we can tag these suggestions ourselves |
|
Yeah, rust-lang/rust#39254 would be great to have.
My goal here right now is to supply a tool that we can use to build such a list :) |
|
Hmm... I started a review of this on some pc... TLDR: yes please! The tests look awesome.
uhm... should be fine on stage2, right? I wanted to have rustc diagnostic json parsing in a separate crate anyway, so compiletest, rustfix and other tools can reuse it. WRT this PR, looks good to me, the old fixtures could probably be left in, but they are not really worth their hazzle. |
The CLI asserts are actually fine to have, I just wanted to move away from submodules and testing against crates. I also had some shim to run this via cargo (basically run cargo-inti in a tmpdir, copy source to main.rs, run cargo fix --yolo), but it was a total hack and didn't add much value overall. |
|
Alright, let's land this now before I leave this hanging for another week. Feel free to continue discussion stuff here, though. |
3 participants
While this is just a quick draft, these tests use rustfix as a library and showcase how it might be used in clippy's test suite to check that suggestions can be applied. (It could e.g. work as an extension to compiletest.)